chore: restore code required to perform sync through bellatrix - #8700
Conversation
Summary of ChangesHello @nflaig, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request restores the necessary code paths to allow syncing through the Bellatrix fork, which was previously noted as a missing capability. The core of the change involves introducing new helper functions to accurately determine if the execution layer is enabled or if the merge transition is complete. These checks are then strategically applied across various modules, including block verification and fork choice, to ensure that execution-related logic is only engaged under the correct conditions. Additionally, the test suite has been updated to remove skips for scenarios that are now properly handled. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request restores the functionality to sync through the Bellatrix fork by reintroducing the isExecutionEnabled check. The changes involve adding this check in various places where execution payloads are handled and re-enabling related spec tests.
My review found a critical issue in the implementation of the new isExecutionEnabled function which would prevent syncing with blinded blocks during the merge transition. I've provided a suggestion to fix this. The rest of the changes look correct and consistent with the PR's goal.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request restores the functionality to sync through the Bellatrix fork by re-introducing checks for isExecutionEnabled() and isMergeTransitionComplete(). The changes correctly add these checks in block processing and fork choice logic, and also re-enable the corresponding spec tests.
My review found a critical issue in the implementation of the new isExecutionEnabled helper function which would prevent syncing with blinded transition blocks. I've left a specific comment with a suggested fix for this.
Performance Report✔️ no performance regression detected Full benchmark results
|
|
deployed to |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #8700 +/- ##
=========================================
Coverage 52.12% 52.12%
=========================================
Files 848 848
Lines 64948 64948
Branches 4783 4783
=========================================
Hits 33853 33853
Misses 31026 31026
Partials 69 69 🚀 New features to boost your workflow:
|
|
🎉 This PR is included in v1.39.0 🎉 |
Motivation
As noted in #8680 (comment) we cannot sync through bellatrix anymore. While I don't think it's a big deal it's simple enough to keep that functionality as that code is pretty isolated and won't get in our way during refactors and with gloas won't be part of the block processing pipeline anymore due to block/payload separation.
Description
Restore code required to perform sync through bellatrix
isExecutionEnabled()andisMergeTransitionComplete()checks during block processing